Skip to content

SHA-256 Hash #1725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

SHA-256 Hash #1725

wants to merge 17 commits into from

Conversation

Alexander-Ger-Reich
Copy link
Contributor

I wrote an add-on that requires the client to provide the SHA-256 hash of the uploaded files and chunks.

@Alexander-Ger-Reich Alexander-Ger-Reich force-pushed the Alexander-Ger-Reich-patch-4 branch from 20e4b85 to e2f1815 Compare May 13, 2025 14:22
@Alexander-Ger-Reich Alexander-Ger-Reich force-pushed the Alexander-Ger-Reich-patch-4 branch from 8b063e6 to d99da0e Compare May 15, 2025 17:58
@Alexander-Ger-Reich
-
fbf52fc 
-
@Alexander-Ger-Reich Alexander-Ger-Reich force-pushed the Alexander-Ger-Reich-patch-4 branch from 85c283e to 774eb29 Compare May 15, 2025 19:14
@Alexander-Ger-Reich Alexander-Ger-Reich force-pushed the Alexander-Ger-Reich-patch-4 branch from 2f73b59 to df8311e Compare May 15, 2025 19:36
@Alexander-Ger-Reich
enhancement
012b77b 
Signed-off-by: Alexander-Ger-Reich <[email protected]>
@Alexander-Ger-Reich Alexander-Ger-Reich force-pushed the Alexander-Ger-Reich-patch-4 branch from df8311e to 012b77b Compare May 15, 2025 19:39
@Alexander-Ger-Reich
Copy link
Contributor Author

DCO is happy

@github-actions GitHub Actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

alperozturk96
alperozturk96 requested changes May 26, 2025
View reviewed changes
Copy link
Contributor

@alperozturk96 alperozturk96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello

Thank you for the PR.

I've suggested a few naming changes to align with the naming conventions used throughout the project for consistency. I will also be testing the PR shortly.

library/src/main/java/com/owncloud/android/lib/resources/files/DownloadFileRemoteOperation.java
@@ -135,8 +138,55 @@ private int downloadFile(OwnCloudClient client, File targetFile) throws IOExcept
transferEncoding = "chunked".equals(transferEncodingHeader.getValue());
}

if (transferred == totalToTransfer || transferEncoding) {
savedFile = true;
if (transferred == totalToTransfer || transferEncoding) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, could you apply the fail fast here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by that?

Copy link
Contributor

@alperozturk96 alperozturk96 May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having nested if-else blocks, you can return early. However, I think the downloadFile() function is too complex for that. We can skip it, if it’s too much work to refactor.

e.g.

Before:

public void checkAge(int age) {
    if (age >= 0) {
        if (age >= 18) {
            System.out.println("User is an adult.");
        } else {
            System.out.println("User is a minor.");
        }
    } else {
        System.out.println("Invalid age.");
    }
}

After:

public void checkAge(int age) {
    if (age < 0) {
        System.out.println("Invalid age.");
        return;
    }

    if (age >= 18) {
        System.out.println("User is an adult.");
        return;
    }
    
    System.out.println("User is a minor.");
}

Alexander-Ger-Reich and others added 6 commits May 27, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alperozturk96 alperozturk96 alperozturk96 requested changes

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
3. to review enhancement feedback-requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Alexander-Ger-Reich @alperozturk96 @AndyScherzinger